-
Notifications
You must be signed in to change notification settings - Fork 15
feat: allow additional custom IAM policy to attached EC2 role #52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds optional inline IAM policy attachment to the default IAM role. Introduces aws_iam_role_policy "custom" in main.tf, created conditionally when var.custom_policy_document is non-empty (count uses length(var.custom_policy_document) > 0 ? 1 : 0). The policy name concatenates module.role_label.id and var.custom_policy_name, attaches to aws_iam_role.default.name, and uses var.custom_policy_document as the policy JSON. Adds two new input variables in variables.tf: custom_policy_document (string, default "") and custom_policy_name (string, default "custom-policy"). Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
variables.tf (2)
216-220: Validate and trim the custom policy JSON to avoid accidental creation with whitespace/invalid JSONCurrently, any non-empty (even whitespace-only) string triggers resource creation and may fail at apply. Add validation and trimspace.
Apply this diff:
variable "custom_policy_document" { description = "JSON policy document for custom permissions to attach to the SSM Agent role. If not provided, no custom policy will be attached." type = string default = "" + validation { + condition = var.custom_policy_document == "" || can(jsondecode(trimspace(var.custom_policy_document))) + error_message = "custom_policy_document must be empty or a valid JSON IAM policy document." + } }
222-226: Constrain policy name to IAM limits and charactersGuard against invalid characters and excessive length for the name component. This prevents surprises when composing the final name in main.tf.
Apply this diff:
variable "custom_policy_name" { description = "Name for the custom policy. Only used if custom_policy_document is provided." type = string default = "custom-policy" + validation { + condition = length(var.custom_policy_name) > 0 && length(var.custom_policy_name) <= 128 && can(regex("^[-_A-Za-z0-9+=,.@]+$", var.custom_policy_name)) + error_message = "custom_policy_name must be 1–128 chars and match [-_A-Za-z0-9+=,.@]." + } }main.tf (1)
142-148: Harden conditional creation and sanitize inputs; add safety check for composed name length
- Use trimspace so whitespace-only input doesn’t create a broken inline policy.
- Trim the policy payload to avoid leading/trailing whitespace.
- Optional: add a lifecycle precondition to ensure the composed name stays within IAM’s 128-char limit.
Apply this diff:
resource "aws_iam_role_policy" "custom" { - count = length(var.custom_policy_document) > 0 ? 1 : 0 + count = length(trimspace(var.custom_policy_document)) > 0 ? 1 : 0 name = "${module.role_label.id}-${var.custom_policy_name}" role = aws_iam_role.default.name - policy = var.custom_policy_document + policy = trimspace(var.custom_policy_document) + lifecycle { + precondition { + condition = length("${module.role_label.id}-${var.custom_policy_name}") <= 128 + error_message = "Composed inline policy name exceeds 128 characters. Consider a shorter custom_policy_name." + } + } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
main.tf(1 hunks)variables.tf(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.tf
⚙️ CodeRabbit configuration file
**/*.tf: You're a Terraform expert who has thoroughly studied all the documentation from Hashicorp https://developer.hashicorp.com/terraform/docs and OpenTofu https://opentofu.org/docs/.
You have a strong grasp of Terraform syntax and prioritize providing accurate and insightful code suggestions.
As a fan of the Cloud Posse / SweetOps ecosystem, you incorporate many of their best practices https://docs.cloudposse.com/best-practices/terraform/ while balancing them with general Terraform guidelines.
Files:
main.tfvariables.tf
🧠 Learnings (1)
📚 Learning: 2024-11-21T13:30:01.588Z
Learnt from: gberenice
PR: masterpointio/terraform-aws-tailscale#41
File: main.tf:0-0
Timestamp: 2024-11-21T13:30:01.588Z
Learning: In this Terraform module (`main.tf`), read permissions (`ssm:GetParameter`) for SSM parameters are managed by the SSM Agent module (`masterpointio/ssm-agent/aws`), so adding `ssm:GetParameter` permissions in the custom `ssm_policy` module is unnecessary.
Applied to files:
main.tfvariables.tf
🔇 Additional comments (1)
main.tf (1)
142-148: Feature addition looks good and is minimally invasiveConditional inline policy on the role aligns with the module’s design and Cloud Posse patterns (label usage, count-based toggle). With the small hardening above, this should be solid.
| } | ||
| } | ||
|
|
||
| variable "custom_policy_document" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why isn't this updating the README on commit? Do we need to add the trunk action to this repo maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I didn't init Trunk in this repo locally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the Git hook didn't block it.
🤖 I have created a release *beep* *boop* --- ## [1.6.0](v1.5.0...v1.6.0) (2025-08-28) ### Features * allow additional custom IAM policy to attached EC2 role ([#52](#52)) ([93a7757](93a7757)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: masterpointbot[bot] <177651640+masterpointbot[bot]@users.noreply.github.com>
what
resource "aws_iam_role_policy" "custom"to allow for a custom policy document to be attached to the role that is associated with the EC2 instance.why
ec2-user, either manually or via theuser_data. This additional IAM policy document will allow that.Summary by CodeRabbit